Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(cactus-core-api): add ISendRequestResultV1<T> for Fujitsu verifier #2724

Conversation

petermetz
Copy link
Contributor

An interface representing the response object of the send sync and send async
request methods on the connection-chain verifier API.

Also great when trying to help bridge the gap between the API returning
results as unknown and the tests needing auto-completion for the properties
to assert for type of data at runtime.

@see {ISocketApiClient}
@see {Verifier<LedgerApiType extends ISocketApiClient>}
@see {IVerifier}

Signed-off-by: Peter Somogyvari [email protected]

refactor(core-api,verifier-client): use unknown instead of any type

  1. Changes the ISocketApiClient and the Verifier type definitions so that
    instead of any they are heavily relying on unknown which helps spotting
    bugs in the code where type checks should be done at runtime but currently
    aren't.

  2. It also helps with spotting breaking changes in the future because if
    the test cases stop compiling it means you've done a breaking API change,
    but they'll always compile if the parameters and the return values are
    using any for their own types.

  3. I've updated a big chunk of the test cases to have explicit type casts
    onto the shapes that are needed for them to compile (without changing
    any of the runtime behavior).

  4. Also made it so that the json2str function of the DriverCommon
    code file now accepts unknown instead of object type which makes one of
    the linter warnings go away as well.

Signed-off-by: Peter Somogyvari [email protected]

Copy link
Contributor

@jagpreetsinghsasan jagpreetsinghsasan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@petermetz petermetz force-pushed the issue-x1-fujitsu-i-send-request-result-v1_t branch from 3fe5346 to 9e51c7f Compare October 5, 2023 21:44
An interface representing the response object of the send sync and send async
request methods on the connection-chain verifier API.

Also great when trying to help bridge the gap between the API returning
results as `unknown` and the tests needing auto-completion for the properties
to assert for type of data at runtime.

@see {ISocketApiClient<BlockType>}
@see {Verifier<LedgerApiType extends ISocketApiClient<unknown>>}
@see {IVerifier}

[skip ci]

Signed-off-by: Peter Somogyvari <[email protected]>
1. Changes the ISocketApiClient and the Verifier type definitions so that
instead of `any` they are heavily relying on `unknown` which helps spotting
bugs in the code where type checks should be done at runtime but currently
aren't.
2. It also helps with spotting breaking changes in the future because if
the test cases stop compiling it means you've done a breaking API change,
but they'll always compile if the parameters and the return values are
using `any` for their own types.

3. I've updated a big chunk of the test cases to have explicit type casts
onto the shapes that are needed for them to compile (without changing
any of the runtime behavior).
4. Also made it so that the json2str function of the DriverCommon
code file now accepts unknown instead of object type which makes one of
the linter warnings go away as well.

Signed-off-by: Peter Somogyvari <[email protected]>
@petermetz petermetz force-pushed the issue-x1-fujitsu-i-send-request-result-v1_t branch from 9e51c7f to 068d36d Compare October 12, 2023 00:23
@petermetz petermetz merged commit 15b8106 into hyperledger-cacti:main Oct 12, 2023
34 of 72 checks passed
@petermetz petermetz deleted the issue-x1-fujitsu-i-send-request-result-v1_t branch October 12, 2023 02:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants